PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

if (thumbnailUrl != null) {
    existing.updateThumbnailUrl(thumbnailUrl);
}

PR Code Suggestions ✨

Explore these optional code suggestions:

 ...
         s3Client.putObject(putObjectRequest,
                 RequestBody.fromInputStream(file.getInputStream(), file.getSize()));
 
         return getFileUrl(objectKey);
     } catch (IOException e) {
-        throw CustomException.internalServerError("S3 업로드 중 오류가 발생했습니다.");
+        throw CustomException.internalServerError("파일 스트림 처리 중 오류가 발생했습니다: " + e.getMessage());
+    } catch (SdkClientException | S3Exception e) {
+        throw CustomException.internalServerError("S3 업로드 중 AWS SDK 오류가 발생했습니다: " + e.getMessage());
     }

-...
-    String ext = switch (contentType != null ? contentType : "") {
-        case "image/png"  -> "png";
-        case "image/jpeg" -> "jpg";
-        case "image/webp" -> "webp";
-        default -> throw CustomException.badRequest("지원하지 않는 이미지 형식입니다: " + contentType);
-    };
-...
+private static final Map<String, String> SUPPORTED_IMAGE_EXTENSIONS = Map.of(
+    "image/png", "png",
+    "image/jpeg", "jpg",
+    "image/webp", "webp"
+);
 
+// ...
+
+private String uploadThumbnailIfPresent(MultipartFile image, Long userId, Long potId) {
+    if (image == null || image.isEmpty()) {
+        return null;
+    }
+    String contentType = image.getContentType();
+    String ext = SUPPORTED_IMAGE_EXTENSIONS.get(contentType);
+    if (ext == null) {
+        throw CustomException.badRequest("지원하지 않는 이미지 형식입니다: " + contentType);
+    }
+    String objectKey = String.format("til-images/%d/%d/%s.%s", userId, potId, UUID.randomUUID(), ext);
+    return s3Service.uploadFile(image, objectKey);
+}
+

Code Review

🔴 Critical

API 하위 호환성 파괴

@PostMapping(consumes = {MULTIPART_FORM_DATA_VALUE, APPLICATION_JSON_VALUE})에서 @RequestPart는 multipart가 아닌 요청에선 동작하지 않습니다. 기존 application/json 클라이언트는 파싱 오류가 발생하므로 APPLICATION_JSON_VALUE를 제거하거나 클라이언트 전환 가이드를 명시해야 합니다.

🟠 Major

1. DB 마이그레이션 스크립트 누락

Til 엔티티에 thumbnail_url 컬럼이 추가됐지만 Flyway/Liquibase 마이그레이션 파일이 없습니다. ddl-auto=validate 환경에서 배포 시 실패합니다.

2. 임시저장 업데이트 시 기존 S3 객체 미삭제

기존 draft에 thumbnailUrl이 있는 상태에서 새 이미지 업로드 시 이전 S3 객체가 삭제되지 않아 고아 객체가 누적됩니다.

3. 파일 크기 검증 누락

uploadThumbnailIfPresent()에서 파일 크기 제한이 없습니다. 서비스 레이어에서 명시적 크기 검증을 추가하거나 spring.servlet.multipart.max-file-size 설정을 확인해주세요.

🔵 Minor

1. S3Config 자격증명 로직 중복

s3Presigner()와 s3Client()에 동일한 credentialsProvider 생성 로직이 중복됩니다. private 헬퍼 메서드로 추출을 권장합니다.

2. 테스트 DisplayName 불일치

S3ServiceTest.uploadFile_unsupportedContentType의 DisplayName이 "400 예외"로 되어 있지만 실제로는 예외 없이 URL을 반환합니다. 이름 수정이 필요합니다.

✅ 긍정적 평가

• micrometer-registry-prometheus → runtimeOnly 변경: 정확한 수정
• uploadFile()의 IOException / S3Exception / SdkClientException 세분화 처리 및 로깅이 잘 되어 있음
• TilServiceTest에 이미지 없음/있음/미지원 타입 3가지 시나리오 추가: 적절한 커버리지

코드리뷰 — 커밋 17bf895 TIL 수정 시 썸네일 S3 업로드

전체적으로 create() · saveDraft()와 동일한 패턴을 적용해 일관성이 좋습니다. 한 가지 수정이 필요한 이슈와 작은 개선 사항 공유드립니다.

🔴 기존 S3 객체 미삭제

update() 시 새 이미지를 업로드하기 전에 기존 thumbnailUrl의 S3 오브젝트를 삭제하지 않습니다. 수정이 반복되면 미사용 오브젝트가 계속 누적되어 스토리지 비용이 발생합니다.

// 개선 예시
String oldUrl = til.getThumbnailUrl();
String newUrl = uploadThumbnailIfPresent(thumbnailImage, userId, til.getPot().getId());
if (oldUrl != null) s3Service.deleteFile(oldUrl);
til.updateThumbnailUrl(newUrl);

현재 S3Service에 deleteFile() 메서드가 없으므로 추가가 필요합니다.

🟡 이중 null 체크

update() 내부에서 thumbnailImage != null && !thumbnailImage.isEmpty()를 먼저 체크한 뒤 uploadThumbnailIfPresent()를 호출하는데, 해당 메서드 내부에도 동일한 가드가 있어 중복입니다. 외부 조건을 제거하고 반환값의 null 여부로 분기하면 create() · saveDraft()와도 패턴이 통일됩니다.

[문제]
TIL 수정 시 이미지를 넣으면 PUT /api/v1/tils/{tilId}에서 413 오류 발생

[원인]

• POST /tils (생성) │ multipart/form-data │ 별도 파일 파트로 전송 ✅
• POST /tils/draft (임시저장) │ multipart/form-data │ 별도 파일 파트로 전송 ✅
• PUT /tils/{tilId} (수정) │ application/json │ 이미지가 base64로 content에 삽입 → JSON 바디 수 MB → 413 ❌

[수정한 파일]

1. TilController.java

• PUT /{tilId}를 multipart/form-data 수락으로 변경
• @RequestBody → @RequestPart("data")
• 선택적 image 파트 추가 (create/draft와 동일한 패턴)

2. TilService.java

• update() 메서드에 MultipartFile thumbnailImage 파라미터 추가
• 이미지가 있으면 S3 업로드 후 thumbnailUrl 갱신

3. application.yml

• multipart 크기 제한 추가: max-file-size: 10MB, max-request-size: 20MB

🔴 Critical — 반드시 수정

1. 기존 JSON 요청이 깨질 가능성이 큼
   TilController.java:37-41, 69-74, 88-92
   consumes에 application/json을 남겨두었지만 파라미터는 @RequestPart("data")로 변경되어 있습니다. @RequestPart는 multipart 요청의 part를 읽는 방식이라, 기존처럼 JSON body를 보내는 클라이언트는 data part가 없어 400/415로 실패할 가능성이 큽니다.
   주석에는 “이미지 없이 JSON만 전송하는 경우에도 정상 동작”이라고 되어 있지만, 현재 코드 구조와 맞지 않습니다.
   권장 수정:

@PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<ApiResponse<TilResponse>> createJson(
        @AuthenticationPrincipal JwtUserDetails userDetails,
        @Valid @RequestBody TilCreateRequest request
) {
    return ResponseEntity.status(HttpStatus.CREATED)
            .body(ApiResponse.success(tilService.create(userDetails.getUserId(), request, null)));
}

@PostMapping(consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
public ResponseEntity<ApiResponse<TilResponse>> createMultipart(
        @AuthenticationPrincipal JwtUserDetails userDetails,
        @RequestPart("data") @Valid TilCreateRequest request,
        @RequestPart(value = "image", required = false) MultipartFile thumbnailImage
) {
    ...
}

update, saveDraft도 동일하게 JSON용 / multipart용을 분리하는 게 안전합니다.

2. thumbnail_url 컬럼 추가에 대한 마이그레이션이 없음
   Til.java:27-29, application.yml:87-90
   Til 엔티티에 새 컬럼이 추가됐지만 DB 마이그레이션 SQL이 없습니다. prod 프로필은 ddl-auto: validate라 운영 DB에 컬럼이 없으면 Hibernate 검증 단계에서 앱이 시작되지 않습니다.

@Column(name = "thumbnail_url")
private String thumbnailUrl;

권장 수정:

ALTER TABLE til
    ADD COLUMN thumbnail_url VARCHAR(2048) NULL;

기존 프로젝트가 src/main/resources/db/*.sql에 수동 마이그레이션 SQL을 두는 흐름이 있으니, 같은 방식으로 추가하는 게 좋아 보입니다.

3. S3 업로드 성공 후 DB 트랜잭션 실패 시 orphan 파일이 남음
   TilService.java:58-64, 66-80, S3Service.java:53-65
   현재 흐름은 DB 저장 전에 S3 업로드를 먼저 수행합니다.

String thumbnailUrl = uploadThumbnailIfPresent(...);
Til til = Til.create(..., thumbnailUrl);
tilRepository.save(til);
syncTags(...);
experienceService.applyWatering(...);

S3 업로드는 성공했는데 이후 tilRepository.save, syncTags, applyWatering 중 하나가 실패하면 DB 트랜잭션은 롤백되지만 S3 파일은 그대로 남습니다.
권장 수정:

• 실패 시 업로드한 objectKey를 삭제하는 보상 로직 추가
• 또는 presigned upload + DB에는 최종 확정된 URL만 저장하는 구조 유지
• 최소한 try/catch로 DB 실패 시 s3Service.deleteFile(objectKey) 호출 가능하도록 uploadFile이 URL뿐 아니라 objectKey도 추적 가능하게 설계

4. 수정/삭제/임시저장 삭제 시 기존 S3 파일이 정리되지 않음
   TilService.java:119-121, 127-136, 153-160, 174-181
   새 썸네일로 수정하면 기존 썸네일 파일이 S3에 남습니다. TIL 삭제, draft 삭제 시에도 thumbnail 파일 삭제가 없습니다.

영향:

• 사용자가 이미지를 여러 번 바꾸면 미사용 파일이 계속 누적
• 삭제한 TIL의 이미지가 public URL로 계속 접근 가능할 수 있음
• 저장 비용과 개인정보/콘텐츠 삭제 정책 측면에서 위험

권장 수정:

• update에서 기존 thumbnailUrl이 있으면 새 업로드 성공 후 기존 파일 삭제
• delete, deleteDraft에서 연결된 thumbnail object 삭제
• URL에서 objectKey를 파싱하기 어렵다면 DB에는 thumbnailUrl뿐 아니라 thumbnailObjectKey 저장도 고려

🟠 High — 가급적 수정

이미지 검증이 Content-Type 헤더만 신뢰함
TilService.java:192-198
현재는 MultipartFile.getContentType()만 보고 확장자를 결정합니다.

case "image/png" -> "png";
case "image/jpeg" -> "jpg";
case "image/webp" -> "webp";

클라이언트가 임의로 image/jpeg를 붙이면 실제 파일 내용이 이미지가 아니어도 S3에 올라갈 수 있습니다.
권장 수정:

• 최소한 파일 시그니처 검사 추가
• 가능하면 이미지 디코딩 검증
• 업로드 후 public으로 제공된다면 특히 필요

6. thumbnail_url 기본 길이 255는 부족할 수 있음
   Til.java:28
   JPA 기본 String 컬럼 길이는 보통 255입니다. S3 URL은 현재 패턴에서는 대체로 들어갈 수 있지만, endpoint/custom domain/CloudFront/path 변경이 생기면 255를 넘길 수 있습니다.
   권장 수정:

@Column(name = "thumbnail_url", length = 2048)
private String thumbnailUrl;

마이그레이션 SQL도 같은 길이로 맞추는 게 좋습니다.

7. public URL 반환 전제가 명확하지 않음
   S3Service.java:78-86
   getFileUrl()은 public URL을 만들어 반환합니다.

return String.format("https://%s.s3.%s.amazonaws.com/%s", bucket, region, objectKey);

버킷이 public-read 정책이 아니면 FE는 URL을 받아도 이미지를 볼 수 없습니다. 기존 presigned upload 흐름도 imageUrl을 반환하고 있어서 같은 전제가 있었을 수 있지만, 이번 PR에서 직접 업로드 썸네일까지 들어오면서 더 중요해졌습니다.
확인 필요:

• 버킷 public read 허용 여부
• CloudFront를 쓰는지 여부
• private bucket이면 presigned GET 또는 CDN URL 정책 필요

8. TIL 이미지 업로드 방식이 두 갈래로 나뉨
   PresignedUrlController.java:27-44, TilService.java:188-200
   기존에는 /tils/image/presigned-url로 본문 이미지를 클라이언트가 직접 업로드하는 구조가 있습니다. 이번 PR은 썸네일만 서버가 직접 업로드합니다.

두 방식이 공존하는 건 가능하지만, 정책이 달라질 수 있습니다.

• 본문 이미지: presigned URL
• 썸네일 이미지: BE 서버 직접 업로드
• contentType 검증 위치 다름
• 파일 삭제 정책 없음
• objectKey 규칙은 유사하지만 책임 주체가 다름

방식 차이가 의도된 것인지 PR 설명에 명확히 적는 게 좋습니다. 가능하면 썸네일도 presigned 방식으로 통일하는 편이 서버 부하와 트랜잭션 정합성 면에서 더 단순합니다.

🟡 Medium — 확인 필요

9. multipart 컨트롤러 테스트가 없음
   TilController.java:37, 69, 88
   서비스 테스트는 추가됐지만, 실제로 깨질 가능성이 큰 부분은 Controller의 바인딩입니다. 특히 @RequestPart("data")는 FE가 data part에 application/json 타입 Blob을 넣어 보내는지에 따라 성공/실패가 갈립니다.
   추가 권장 테스트:

• 기존 JSON POST /api/v1/tils 성공
• multipart POST /api/v1/tils 성공
• multipart에서 data part 누락 시 400
• image/gif 업로드 시 400
• PUT /api/v1/tils/{id}, POST /api/v1/tils/draft도 동일

10. TilServiceTest가 실제 저장된 thumbnailUrl을 검증하지 않음
    TilServiceTest.java:108-124
    현재 테스트는 s3Service.uploadFile() 호출 여부만 봅니다. 그런데 tilRepository.save(any(Til.class))가 미리 만든 til을 반환하므로, 실제 생성된 Til에 thumbnailUrl이 들어갔는지는 확인하지 못합니다.
    권장 수정:

• ArgumentCaptor로 저장 대상 캡처
• captured.getThumbnailUrl() 검증
• TilResponse.thumbnailUrl()도 검증

11. S3 업로드 실패가 모두 500으로만 내려감
    S3Service.java:66-75
    S3 권한 오류, 버킷 없음, 파일 스트림 오류, 네트워크 오류가 모두 500 계열로 내려갑니다. 서버 내부 오류로 처리하는 건 틀리진 않지만, 운영 분석을 위해 에러 메시지/코드를 조금 더 구분하면 좋습니다.
    예:

• 인증/권한 문제: 로그에 statusCode, awsErrorCode 포함
• 파일 스트림 문제: 400 또는 500 정책 명확화
• 연결 실패: 503도 고려 가능

12. 이미지 삭제 API/교체 정책이 없음
    기능상 썸네일을 추가할 수는 있지만 제거할 수는 없습니다. update는 새 파일이 있을 때만 교체하고, null 또는 별도 플래그로 기존 썸네일 삭제를 요청하는 방식이 없습니다.
    FE에서 “썸네일 제거” 기능을 제공할 계획이 있다면 API 계약을 먼저 정하는 게 좋습니다.
13. 파일 크기 제한은 전체 multipart 기준만 있고 썸네일 정책은 없음
    application.yml:4-7

max-file-size: 10MB
max-request-size: 20MB

10MB 이미지는 썸네일로는 꽤 큽니다. 본문 이미지 정책과 썸네일 정책이 다를 수 있으니 썸네일은 별도 제한을 두는 것도 고려할 만합니다.
예:

• 썸네일: 2MB 이하
• 본문 이미지: 10MB 이하

14. Metrics 변경이 PR 범위 밖으로 보임
    build.gradle:71-73, MetricsConfig.java:1-19
    S3 업로드 PR인데 micrometer-jvm-extras 의존성과 MetricsConfig가 추가되어 있습니다. 기능적으로 직접 관련이 없어 리뷰 범위가 흐려집니다.

implementation 'io.github.mweirauch:micrometer-jvm-extras:0.2.2'

권장:

• 메트릭 보강은 별도 PR로 분리
• 이 PR에서는 S3/TIL 썸네일 관련 변경만 유지

15. micrometer-registry-prometheus를 implementation에서 runtimeOnly로 바꾼 이유가 PR에 없음
    build.gradle:71-72
    이 변경 자체가 틀렸다고 보긴 어렵지만, Prometheus 관련 설정은 배포/관측에 영향을 줄 수 있습니다. S3 업로드 PR에 포함되면 원인 추적이 어려워집니다.

• 잘 몰라서 AI 돌린거라...참고만 하셔도 좋을거 같아서 남겼어요...
• 완석님이 스킵하고 싶으신 부분은 스킵하셔도 될거 같습니다!